Skip to content

.Net: Add RetainArgumentTypes support to FunctionCallContentBuilder#13142

Closed
ioshm wants to merge 5 commits into
microsoft:mainfrom
ioshm:retain-args-fcc
Closed

.Net: Add RetainArgumentTypes support to FunctionCallContentBuilder#13142
ioshm wants to merge 5 commits into
microsoft:mainfrom
ioshm:retain-args-fcc

Conversation

@ioshm

@ioshm ioshm commented Sep 17, 2025 •
edited
Loading

Copy link
Copy Markdown

Motivation and Context

Before this patch, the FunctionCallContentBuilder did not support retaining the argument types. We need this to support retained argument types when not using auto invoke function choices, i.e: a custom agent loop.

Figured this was small enough to not warrant an issue.

Description

Modified the experimental constructor to include a bool retainArgumentTypes parameter. I decided to update the existing experimental constructor to avoid polluting the class with extra constructors. Not sure if this is considered breaking.

Contribution Checklist

@ioshm ioshm requested a review from a team as a code owner September 17, 2025 14:48
@moonbox3 moonbox3 added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel kernel.core labels Sep 17, 2025
@ioshm

ioshm commented Sep 17, 2025

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@ioshm
.Net: Add RetainArgumentTypes support to FunctionCallContentBuilder
9bf7fae 
Before this patch, the `FunctionCallContentBuilder` did not support retaining the
argument types. We need this to support retained argument types when not using auto
invoke function choices.
@ioshm

ioshm commented Sep 25, 2025

Copy link
Copy Markdown
Author

The FunctionUsageMetricsAreCapturedByTelemetryAsExpected test seems flaky and races with other tests.

Can reproduce by spamming the following until it races:

dotnet test dotnet\src\Connectors\Connectors.AzureOpenAI.UnitTests --filter "(FullyQualifiedName=SemanticKernel.Connectors.AzureOpenAI.UnitTests.KernelCore.KernelTests.FunctionUsageMetricsAreCapturedByTelemetryAsExpected) | (FullyQualifiedName=SemanticKernel.Connectors.AzureOpenAI.UnitTests.Services.AzureOpenAIChatCompletionServiceTests.GetChatMessageContentsUsesPromptAndSettingsCorrectlyAsync)" --no-build
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v3.1.3+b1b99bdeb3 (64-bit .NET 8.0.17)
[xUnit.net 00:00:00.09]   Discovering: SemanticKernel.Connectors.AzureOpenAI.UnitTests
[xUnit.net 00:00:00.21]   Discovered:  SemanticKernel.Connectors.AzureOpenAI.UnitTests
[xUnit.net 00:00:00.23]   Starting:    SemanticKernel.Connectors.AzureOpenAI.UnitTests
[xUnit.net 00:00:00.46]     SemanticKernel.Connectors.AzureOpenAI.UnitTests.KernelCore.KernelTests.FunctionUsageMetricsAreCapturedByTelemetryAsExpected [FAIL]
[xUnit.net 00:00:00.46]       Assert.Contains() Failure: Item not found in collection
[xUnit.net 00:00:00.46]       Collection: [55]
[xUnit.net 00:00:00.46]       Not found:  12
[xUnit.net 00:00:00.46]       Stack Trace:
[xUnit.net 00:00:00.46]         C:\Dayforce\repos\semantic-kernel\dotnet\src\Connectors\Connectors.AzureOpenAI.UnitTests\KernelCore\KernelTests.cs(97,0): at SemanticKernel.Connectors.AzureOpenAI.UnitTests.KernelCore.KernelTests.<>c__DisplayClass6_0.<FunctionUsageMetricsAreCapturedByTelemetryAsExpected>b__2(Instrument instrument, Object state)
[xUnit.net 00:00:00.46]            at System.Diagnostics.Metrics.MeterListener.Dispose()
[xUnit.net 00:00:00.46]         C:\Dayforce\repos\semantic-kernel\dotnet\src\Connectors\Connectors.AzureOpenAI.UnitTests\KernelCore\KernelTests.cs(117,0): at SemanticKernel.Connectors.AzureOpenAI.UnitTests.KernelCore.KernelTests.FunctionUsageMetricsAreCapturedByTelemetryAsExpected()
[xUnit.net 00:00:00.46]         --- End of stack trace from previous location ---
[xUnit.net 00:00:00.46]   Finished:    SemanticKernel.Connectors.AzureOpenAI.UnitTests
  Connectors.AzureOpenAI.UnitTests test failed with 1 error(s) (1.5s)
    C:\Dayforce\repos\semantic-kernel\dotnet\src\Connectors\Connectors.AzureOpenAI.UnitTests\KernelCore\KernelTests.cs(97): error TESTERROR:
      SemanticKernel.Connectors.AzureOpenAI.UnitTests.KernelCore.KernelTests.FunctionUsageMetricsAreCapturedByTelemetryAsExpected (189ms): Error Message: Assert.Contains() Failure: Item not found in collection
      Collection: [55]
      Not found:  12
      Stack Trace:
         at SemanticKernel.Connectors.AzureOpenAI.UnitTests.KernelCore.KernelTests.<>c__DisplayClass6_0.<FunctionUsageMetricsAreCapturedByTelemetryAsExpected>b__2(Instrument instrument, Object state) in C:\Day
      force\repos\semantic-kernel\dotnet\src\Connectors\Connectors.AzureOpenAI.UnitTests\KernelCore\KernelTests.cs:line 97
         at System.Diagnostics.Metrics.MeterListener.Dispose()
         at SemanticKernel.Connectors.AzureOpenAI.UnitTests.KernelCore.KernelTests.FunctionUsageMetricsAreCapturedByTelemetryAsExpected() in C:\Dayforce\repos\semantic-kernel\dotnet\src\Connectors\Connectors.A
      zureOpenAI.UnitTests\KernelCore\KernelTests.cs:line 117
      --- End of stack trace from previous location ---

Test summary: total: 2, failed: 1, succeeded: 1, skipped: 0, duration: 1.4s
Build failed with 1 error(s) in 1.9s

Can also reproduce from VS by running these two tests in parallel until failure.

auto-merge was automatically disabled September 25, 2025 15:41

Head branch was pushed to by a user without write access

@ioshm

ioshm commented Sep 25, 2025

Copy link
Copy Markdown
Author

Find a fix in e08e4d1.

This bit of code can be triggered concurrently (from two parallel tests). The Add call on the List<long> is not thread-safe, giving raise to a lost update problem when there is more than 1 concurrent writer.

semantic-kernel/dotnet/src/Connectors/Connectors.AzureOpenAI.UnitTests/KernelCore/KernelTests.cs

Lines 83 to 90 in e08e4d1

listener.SetMeasurementEventCallback<long>((instrument, measurement, tags, state) =>
{
if (instrument.Name == "semantic_kernel.function.invocation.token_usage.prompt" ||
instrument.Name == "semantic_kernel.function.invocation.token_usage.completion")
{
measurements[instrument.Name].Add(measurement);
}
});

Fix the problem by using a thread-safe collection, ConcurrentBag<long>.

@ioshm ioshm requested a review from SergeyMenshykh October 11, 2025 17:28
@westey-m westey-m moved this to Community PR in Agent Framework Apr 29, 2026
@SergeyMenshykh SergeyMenshykh moved this from Community PR to Backlog in Agent Framework May 1, 2026
@SergeyMenshykh

SergeyMenshykh commented May 12, 2026

Copy link
Copy Markdown
Contributor

Closing it for now. @ioshm feel free to reopen it if it's still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SergeyMenshykh SergeyMenshykh Awaiting requested review from SergeyMenshykh
1 more reviewer
@markwallace-microsoft markwallace-microsoft markwallace-microsoft approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@SergeyMenshykh SergeyMenshykh

Labels

kernel.core kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code

Projects

Agent Framework
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ioshm @SergeyMenshykh @markwallace-microsoft @moonbox3 @westey-m